Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

benchmarks for create_client #1392

Merged
merged 2 commits into from
Dec 16, 2024
Merged

benchmarks for create_client #1392

merged 2 commits into from
Dec 16, 2024

Conversation

insipx
Copy link
Contributor

@insipx insipx commented Dec 6, 2024 •
edited
Loading

Benchmarks against dev_grpc:

Screenshot 2024-12-13 at 11 38 00 AM
Screenshot 2024-12-13 at 11 37 52 AM

Flamegraph:
tracing-flamegraph

According to flamegraph, we're spending the most time in tonic::Endpoint::connect

We could use connect_lazy but that just offloads it to the next api call, which in this case is related to initializing the identity (get_inbox_ids).

In another PR I'd like to either:

  • Separate gRPC channel connection from client creation. Allow SDKs to initialize the backend connection somewhere separate from client creation
  • use connect_lazy but would require a refactor of client creation to somehow defer identity verification to the background. Seems like maybe a messy/big refactor path

@insipx insipx mentioned this pull request Dec 6, 2024
Merged
@insipx Graphite App
Copy link
Contributor Author

insipx commented Dec 6, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@insipx insipx mentioned this pull request Dec 6, 2024
Closed
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from f8d41b5 to d0b1dae Compare December 6, 2024 20:01
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 22985f7 to 2a6da02 Compare December 6, 2024 20:01
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from d0b1dae to 43a5099 Compare December 6, 2024 20:15
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 2a6da02 to 81ed359 Compare December 6, 2024 20:15
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 43a5099 to de7f9bb Compare December 6, 2024 20:32
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 81ed359 to 80a2006 Compare December 6, 2024 20:32
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from de7f9bb to da408b0 Compare December 6, 2024 20:44
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 80a2006 to 570bff5 Compare December 6, 2024 20:44
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from da408b0 to bdfae14 Compare December 9, 2024 18:05
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 570bff5 to e6c95c8 Compare December 9, 2024 18:05
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from bdfae14 to abd8aeb Compare December 9, 2024 18:15
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from e6c95c8 to aa09510 Compare December 9, 2024 18:15
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from abd8aeb to c432f08 Compare December 10, 2024 21:31
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from aa09510 to 60a44dc Compare December 10, 2024 21:31
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from c432f08 to 91dbcd4 Compare December 11, 2024 15:07
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 60a44dc to 7334ebe Compare December 11, 2024 15:07
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 91dbcd4 to 3df89c2 Compare December 11, 2024 16:24
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 7334ebe to 8067f17 Compare December 11, 2024 16:24
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 3df89c2 to 919f767 Compare December 11, 2024 16:25
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 8067f17 to defd953 Compare December 11, 2024 16:25
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 919f767 to 615d449 Compare December 11, 2024 21:06
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from defd953 to 9d8cc2c Compare December 11, 2024 21:06
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 615d449 to 78b7065 Compare December 11, 2024 21:14
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 9d8cc2c to cd99a8f Compare December 11, 2024 21:14
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 78b7065 to 659acd1 Compare December 11, 2024 21:17
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from cd99a8f to 6c966d7 Compare December 11, 2024 21:17
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 659acd1 to 718950b Compare December 11, 2024 21:22
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 6c966d7 to 40fc431 Compare December 11, 2024 21:22
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 718950b to 9b9da95 Compare December 11, 2024 23:12
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 40fc431 to f1741b9 Compare December 11, 2024 23:12
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 9b9da95 to 0b8d1fe Compare December 11, 2024 23:19
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from f1741b9 to ecd5f83 Compare December 11, 2024 23:19
@insipx insipx force-pushed the 11-30-common_crate_for_wasm_shims branch from 0b8d1fe to fd98665 Compare December 11, 2024 23:31
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch 2 times, most recently from 3835116 to dae833b Compare December 12, 2024 16:23
Base automatically changed from 11-30-common_crate_for_wasm_shims to main December 12, 2024 16:38
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch 5 times, most recently from e1d8afd to 02e809c Compare December 13, 2024 16:56
@insipx insipx marked this pull request as ready for review December 13, 2024 16:56
@insipx insipx requested a review from a team as a code owner December 13, 2024 16:56
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 02e809c to c3caaf7 Compare December 13, 2024 16:57
@neekolas
Copy link
Contributor

neekolas commented Dec 13, 2024 •
edited
Loading

use connect_lazy but would require a refactor of client creation to somehow defer identity verification to the background. Seems like maybe a messy/big refactor path

While it would improve performance, I'm not sure it's desirable. If there is something wrong with your identity or credentials we would want to bubble that up to the caller as soon as possible.

@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from c3caaf7 to ebc2ef5 Compare December 13, 2024 20:17
insipx
insipx commented Dec 13, 2024
View reviewed changes
xmtp_mls/src/groups/device_sync.rs
}

#[cfg(any(test, feature = "test-utils"))]
pub struct WorkerHandle {
Copy link
Contributor Author

@insipx insipx Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was giving a bunch of unused in bindings_node, since it doesn't rely on test-utils in their dev dependencies. I moved it to utils/test/mod.rs to make it easier to manage the dependencies specific to the WorkerHandle and those in non-test stuff. Re-exported here for ease of use

@insipx
Copy link
Contributor Author

insipx commented Dec 13, 2024 •
edited
Loading

yeah definitely leaning towards just pulling Tonic out of create_client/making it an `Option'

@insipx insipx mentioned this pull request Dec 13, 2024
Merged
@insipx insipx force-pushed the 12-06-benchmarks_for_create_client branch from 5f0810a to 49ef097 Compare December 13, 2024 22:25
@insipx insipx merged commit 97eca90 into main Dec 16, 2024
14 checks passed
@insipx insipx deleted the 12-06-benchmarks_for_create_client branch December 16, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neekolas neekolas neekolas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@insipx @neekolas